Skip to content

Fix bluetooth shell issue #90065

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 28, 2025

Conversation

CanWang001
Copy link
Contributor

@CanWang001 CanWang001 commented May 16, 2025

This PR contains three patches:

  1. Remove redundant spaces in string.
  2. Fix issue that BR security level cannot be set to 4 by shell command
  3. Fix issue that BR connection is not selected when multiple connections exist and then one of connections is disconnected.

This string contains two consecutive spaces. Remove one of them.

Signed-off-by: Can Wang <[email protected]>
Host stack supports to set BR security level to 4 but the security level
cannot be set to 4 by the shell command.

Update the code to support BR security level 4.

Signed-off-by: Can Wang <[email protected]>
@CanWang001 CanWang001 marked this pull request as ready for review May 17, 2025 14:53
@github-actions github-actions bot added area: Bluetooth area: Bluetooth Host Bluetooth Host (excluding BR/EDR) labels May 17, 2025
LE and BR connection have already been established, after that, LE
disconnection occurs, BR connection will not be selected as the next
default connection.

Fix this issue by searching for both BR and LE after disconnection
occurs.

Signed-off-by: Can Wang <[email protected]>
@CanWang001 CanWang001 force-pushed the fix_bt_shell_issue branch from 5b26acd to ab43e18 Compare May 19, 2025 11:42
@CanWang001 CanWang001 requested a review from Thalley May 19, 2025 11:47
Copy link

@CanWang001
Copy link
Contributor Author

Is there anyone else who can help review this? Thanks

bt_conn_unref(default_conn);
default_conn = NULL;

/* If we are connected to other devices, set one of them as default */
bt_conn_foreach(BT_CONN_TYPE_LE, disconnected_set_new_default_conn_cb, NULL);
bt_conn_foreach(info.type, disconnected_set_new_default_conn_cb, NULL);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be more accurate like this?:

Suggested change
bt_conn_foreach(info.type, disconnected_set_new_default_conn_cb, NULL);
bt_conn_foreach(BT_CONN_TYPE_LE | BT_CONN_TYPE_BR, disconnected_set_new_default_conn_cb, NULL);

I believe it's more accurate because we don't really care about what the type of the disconnected connection is for the purpose of selecting a different connection.

I don't know what the difference between BT_CONN_TYPE_BR and BT_CONN_TYPE_SCO is, so it may be SCO is correct instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi alwa-nordic, as the comments from Thalley, he expects to set a new default conn with the same type first.
Consider a requirement: If there are 4 connections: 2 LE and 2 BR/EDR, and the disconnected one is a BR/EDR, and we want to select the other BR/EDR connection.

SCO disconnection will not reach this callback and will use its own callback.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I'll defer to @Thalley's decision.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively we could remove the "select next" feature.

The reasoning is that if I was doing some GATT operations on an LE default_conn, and then it disconnects, then I'd assume I could either continue with GATT operations, or not do any operations.

Perhaps the "select next" feature isn't that useful for multiple different types of connections?

Copy link
Contributor Author

@CanWang001 CanWang001 May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this PR #90123 is merged, and we use bt select and br select shell command to select different types of connections. After that, the "select next" may be removed.
Anyway, the "select next" provide the possibility to be unnecessary to enter bt select or br select.

@kartben kartben merged commit a95f900 into zephyrproject-rtos:main May 28, 2025
29 checks passed
Copy link

Hi @CanWang001!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Bluetooth Host Bluetooth Host (excluding BR/EDR) area: Bluetooth
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants